Content picker search with start node configured not taking user start nodes into account #19871
Conversation
…t nodes into account (#19800) * Fix users being able to see nodes they don't have access to when using the picker search * Readability and naming improvements * Additional fixes * Adjust tests * Additional fixes * Small improvement * Replaced the root ids with constants * Update src/Umbraco.Web.BackOffice/Trees/MemberTreeController.cs Co-authored-by: Andy Butland <[email protected]> --------- Co-authored-by: Andy Butland <[email protected]> # Conflicts: # src/Umbraco.Examine.Lucene/BackOfficeExamineSearcher.cs # src/Umbraco.Web.BackOffice/Trees/ContentTreeController.cs # src/Umbraco.Web.BackOffice/Trees/MediaTreeController.cs # src/Umbraco.Web.BackOffice/Trees/MemberTreeController.cs # tests/Umbraco.Tests.Integration/Umbraco.Examine.Lucene/UmbracoExamine/BackOfficeExamineSearcherTests.cs
There was a problem hiding this comment.
Pull Request Overview
This PR cherry-picks a fix from V16 and addresses an issue where the "Ignore user start nodes" setting in content picker search was not being properly respected. The fix refactors the path filtering logic to correctly handle user start nodes and search scope restrictions.
- Refactors the
AppendPathmethod to properly handle user start node permissions and search filtering - Simplifies constructor dependencies by removing unused parameters (
IUmbracoMapper,IPublishedUrlProvider) - Updates test helper method to accept
ignoreUserStartNodesparameter for better test coverage
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| BackOfficeExamineSearcher.cs | Major refactoring of path filtering logic and constructor simplification with proper user start node handling |
| BackOfficeExamineSearcherTests.cs | Updated test helper method to support configurable ignoreUserStartNodes parameter |
|
This looks good to me @lauraneto, but I think the issue you raised is worth fixing, at last on the server-side as part of this PR. I had a look into the code and think it's not too big an addition. Looks like You can then do: And pass the value into Will need the same on the For reference if you look at Then of course we need to pass the data type Id from the client. I've not looked to see how easy that would be, but hopefully not too bad. If you can fix it too, please do - otherwise let's create an issue for someone else to look at. |
...co.Tests.Integration/Umbraco.Examine.Lucene/UmbracoExamine/BackOfficeExamineSearcherTests.cs
Outdated
Show resolved
Hide resolved
…to get `ignoreUserStartNodes` value
There was a problem hiding this comment.
This looks to be working as expected in testing, so think we can merge this in now as it's resolved front-end and back-end. Good catch again and nice work.
I pushed one update that I believe should resolve the final breaking change that was being flagged with the controller methods. If that looks good to you and you don't have anything further to add, please feel free to merge.
lauraneto
deleted the
v16/bugfix/Content-picker-search-with-start-node-configured-not-taking-user-start-nodes-into-account
branch
Cherry pick of #19800 to V16 + adding new constructor without obsolete and unused parameters.
During testing I found that "Ignore user start nodes" is always
falsein theBackOfficeExamineSearcher.Search()method, even when it is set totruein the data type.In V13 the data type id was provided in the search query, so that the setting could be retrieved, but that does not seem supported by the new search endpoint. Not sure if this should be looked at in this PR or as a different issue.